Fix clicking on notification does not restore activity #2979
Conversation
ne0rrmatrix
requested review from
TheCodeTraveler
and removed request for
Copilot
There was a problem hiding this comment.
Pull request overview
This pull request fixes notification click behavior in the MediaControlsService for Android by adding a content intent that properly restores the main app activity when the user taps the foreground service notification.
Key changes:
- Added
CreateActivityPendingIntent()helper method to construct the appropriate PendingIntent - Integrated the PendingIntent into the notification builder via
SetContentIntent()
TheCodeTraveler
removed
the
needs discussion
| var packageName = Platform.AppContext.PackageName ?? throw new InvalidOperationException("PackageName cannot be null"); | ||
| var packageManager = Platform.AppContext.PackageManager ?? throw new InvalidOperationException("PackageManager cannot be null"); | ||
| var launchIntent = packageManager.GetLaunchIntentForPackage(packageName) ?? throw new InvalidOperationException("Launch intent cannot be null"); |
There was a problem hiding this comment.
According to the project's coding guidelines, null checking should use the is pattern instead of the null-coalescing operator with throw. Consider refactoring these null checks to use pattern matching for consistency with the codebase standards.
For example, instead of:
var packageName = Platform.AppContext.PackageName ?? throw new InvalidOperationException("PackageName cannot be null");Use:
if (Platform.AppContext.PackageName is not { } packageName)
{
throw new InvalidOperationException("PackageName cannot be null");
}This pattern should be applied to all three null checks in this method (PackageName, PackageManager, and the launch intent).
| return PendingIntent.GetActivity(Platform.AppContext, 0, launchIntent, flags) | ||
| ?? throw new InvalidOperationException("PendingIntent cannot be null"); |
There was a problem hiding this comment.
According to the project's coding guidelines, null checking should use the is pattern instead of the null-coalescing operator with throw. Consider refactoring this null check to use pattern matching for consistency with the codebase standards.
For example, instead of:
return PendingIntent.GetActivity(Platform.AppContext, 0, launchIntent, flags) ?? throw new InvalidOperationException("PendingIntent cannot be null");Use:
if (PendingIntent.GetActivity(Platform.AppContext, 0, launchIntent, flags) is not { } pendingIntent)
{
throw new InvalidOperationException("PendingIntent cannot be null");
}
return pendingIntent;|
|
||
| launchIntent.SetFlags(ActivityFlags.ClearTop | ActivityFlags.SingleTop); | ||
|
|
||
| var flags = PendingIntentFlags.UpdateCurrent | PendingIntentFlags.Immutable; |
There was a problem hiding this comment.
The PendingIntentFlags.Immutable flag was introduced in Android 12 (API 31), but this service supports Android 26+ (as indicated by the [SupportedOSPlatform("Android26.0")] attribute). Using this flag unconditionally will cause runtime errors on Android versions 26-30.
Consider adding a version check to use the appropriate flags:
var flags = OperatingSystem.IsAndroidVersionAtLeast(31)
? PendingIntentFlags.UpdateCurrent | PendingIntentFlags.Immutable
: PendingIntentFlags.UpdateCurrent;Alternatively, if Android 12+ is now the minimum supported version, update the SupportedOSPlatform attribute on the class accordingly.
| var flags = PendingIntentFlags.UpdateCurrent | PendingIntentFlags.Immutable; | |
| var flags = OperatingSystem.IsAndroidVersionAtLeast(31) | |
| ? PendingIntentFlags.UpdateCurrent | PendingIntentFlags.Immutable | |
| : PendingIntentFlags.UpdateCurrent; |
3 participants
Description of Change
This pull request improves the behavior of the foreground service notification in the
MediaControlsServicefor Android by ensuring that tapping the notification brings the user back to the main app activity. The changes introduce a helper method to create the appropriatePendingIntentand attach it to the notification.Enhancement to notification interaction:
MediaControlsServicenow includes a content intent, so tapping the notification will launch (or bring to front) the main app activity. This is achieved by creating aPendingIntentusing the newCreateActivityPendingIntent()method and setting it withSetContentIntenton the notification builder.Internal code improvements:
CreateActivityPendingIntent()method that safely constructs aPendingIntentto launch the main activity, handling potential null references and setting intent flags to ensure correct activity behavior.Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
Fixes a long standing issue where clicking on notification after leaving app does not restore app. This fixes that issue and restores expected default android behavior.